Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backend] Add Cookies and Surface with Privacy Notices #3572

Merged
merged 28 commits into from
Jun 21, 2023

Conversation

pattisdr
Copy link
Contributor

@pattisdr pattisdr commented Jun 15, 2023

Closes #3477
❗ Dependent on Fideslang release https://github.com/ethyca/fideslang/pull/115
❗ Contains migration; check downrev before merge

Description Of Changes

As a privacy admin, I want to document the cookies utilized by a system to deliver its services and functionalities, so that they can be mapped to privacy notices and operated on by the privacy experiences for consent related opt-in/out and so that I have a record of cookies for compliance purposes.

Code Changes

  • Add a Cookies table with a FK to both privacy_declaration and system, in case the privacy declaration gets deleted, we still have cookies that are unassociated with a specific data use and still connected to the system.
  • Cookies are currently specified under privacy_declarations when creating/updating systems via the API
  • Add an upsert_cookies method that is called when privacy declarations are created/updated
  • Add a PrivacyNotice property that calculates at runtime which cookies are relevant based on data use
    • Relevant if a data use matches exactly, or a system's data use is a child of a privacy notice's data use, but not the other way around.

Steps to Confirm

  • Make a POST to create a system, with a list of cookies specified under the relevant privacy declaration(s). Confirm cookies are in the response under both the privacy declaration, and the system
    POST {{host}}/system
{
   "fides_key":"test_systems",
   "name":"Test system",
   "organization_fides_key":"default_organization",
   "system_type": "system",
   "privacy_declarations":[
      {
         "name":"Collect data for marketing",
         "data_categories":[
            "user.device.cookie_id"
         ],
         "data_use":"personalize",
         "data_qualifier":"aggregated.anonymized.unlinked_pseudonymized.pseudonymized.identified",
         "data_subjects":[
            "customer"
         ],
         "cookies":[
            {
               "name":"test_cookie",
               "path":"/"
            },
              {
               "name":"test_cookie",
               "path":"/"
            }
         ]
      }
   ]
}
  • Create a privacy notice with the same data use as your privacy declaration above. POST {{host}}/privacy-notice. Verify that the cookie created above is displayed in the response. If data uses don't overlap with any privacy declaration data uses, the cookies should return as an empty list.
[
   {
      "name":"Profiling",
      "notice_key": "profiling",
      "regions":[
         "us_va"
      ],
      "description":"Making a decision solely by automated means.",
      "consent_mechanism":"opt_in",
      "data_uses":[
         "personalize"
      ],
      "enforcement_level":"system_wide",
      "has_gpc_flag":false,
      "displayed_in_overlay":true
   }
]

Pre-Merge Checklist

pattisdr added 5 commits June 14, 2023 18:51
…and System and an upsert_cookies method that is called when creating/updating privacy declarations.
…s this can't be specified on the system directly currently.

- Fix bug where path/domain aren't being saved on create.
- Add system endpoints to postman collection with new cookie fields.
- Add database annotations.
@cypress
Copy link

cypress bot commented Jun 15, 2023

Passing run #2867 ↗︎

0 4 0 0 Flakiness 0
⚠️ You've recorded test results over your free plan limit.
Upgrade your plan to view test results.

Details:

Merge d283aab into 358b324...
Project: fides Commit: 40c68cbf62 ℹ️
Status: Passed Duration: 01:13 💡
Started: Jun 21, 2023 10:16 PM Ended: Jun 21, 2023 10:17 PM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

requirements.txt Outdated Show resolved Hide resolved
@pattisdr pattisdr marked this pull request as ready for review June 15, 2023 22:30
pattisdr added 3 commits June 16, 2023 12:01
Conflicts:
src/fides/api/ctl/database/crud.py
src/fides/api/ctl/database/system.py
src/fides/api/ctl/migrations/versions/5307999c0dac_remove_deprecated_data_uses_for_.py
src/fides/api/ctl/schemas/system.py
src/fides/api/ctl/sql_models.py
src/fides/api/models/privacy_notice.py
tests/conftest.py
tests/ctl/core/test_system.py
Copy link
Contributor

@allisonking allisonking left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looking good! going to start integrating with the frontend, will report back!

src/fides/api/db/system.py Show resolved Hide resolved
src/fides/api/models/privacy_notice.py Show resolved Hide resolved
pattisdr and others added 8 commits June 19, 2023 08:36
@ThomasLaPiana
Copy link
Contributor

@pattisdr it looks like the fideslang issues are fixed! The final database URL failure is probably related to a Pydantic version change, I can keep poking at that too 😄

@codecov
Copy link

codecov bot commented Jun 21, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.03 🎉

Comparison is base (358b324) 87.06% compared to head (d283aab) 87.10%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3572      +/-   ##
==========================================
+ Coverage   87.06%   87.10%   +0.03%     
==========================================
  Files         310      310              
  Lines       18996    19031      +35     
  Branches     2432     2437       +5     
==========================================
+ Hits        16539    16577      +38     
+ Misses       2028     2026       -2     
+ Partials      429      428       -1     
Impacted Files Coverage Δ
src/fides/api/db/crud.py 74.48% <ø> (ø)
src/fides/api/db/system.py 83.49% <100.00%> (+3.03%) ⬆️
src/fides/api/models/privacy_notice.py 99.13% <100.00%> (+0.01%) ⬆️
src/fides/api/models/sql_models.py 97.81% <100.00%> (+0.09%) ⬆️
src/fides/api/schemas/dataset.py 94.64% <100.00%> (ø)
src/fides/api/schemas/privacy_notice.py 100.00% <100.00%> (ø)
src/fides/api/schemas/privacy_request.py 100.00% <100.00%> (ø)
src/fides/api/schemas/system.py 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@ThomasLaPiana
Copy link
Contributor

ThomasLaPiana commented Jun 21, 2023

@pattisdr the docs failure here is expected, it'll fail because the pip for fideslang needs git and that container doesn't have it

The other Pytest failure is weird to me...I think you recently wrote that so maybe you can confirm if it is intermittent?

The mypy thing is new it seems

@pattisdr
Copy link
Contributor Author

pattisdr commented Jun 21, 2023

@ThomasLaPiana this mypy issue we've been running into across all our branches - isort will move that ignore down a line, so it's no longer ignoring the correct line for mypy. What fixes it for one check will break it for the other

The other Pytest failure is weird to me...I think you recently wrote that so maybe you can confirm if it is intermittent?

It is intermittent but I need to dig into it more. I worry there's something I don't understand about async sessions. I haven't seen it failing on 3.8 or 9 but 3.10

EDIT: Maybe it has to do with system.privacy_declarations order being inconsistent?
EDIT: Ah, there are some new mypy failures, I think it was the pydantic change? I thought it was happening outside this branch at first.

pattisdr and others added 4 commits June 21, 2023 08:56
src/fides/api/api/v1/endpoints/dataset_endpoints.py
src/fides/api/models/datasetconfig.py
src/fides/api/util/data_category.py
tests/ctl/core/test_system.py
tests/ops/api/v1/endpoints/test_dataset_endpoints.py
Copy link
Contributor

@allisonking allisonking left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🍪

@pattisdr pattisdr merged commit f9c8200 into main Jun 21, 2023
@pattisdr pattisdr deleted the fides_3478_system_cookies branch June 21, 2023 23:12
pattisdr added a commit that referenced this pull request Jun 22, 2023
 - Backend: Add a Cookies table with FK's to PrivacyDeclaration and System.  Surface cookies on privacy notices, calculated at runtime
- Frontend: Add cookie input field on system data use tab
- Frontend: Have `fides-js` and privacy center delete cookies associated with notices that were opted out of 


Co-authored-by: Thomas <thomas.lapiana+github@pm.me>
Co-authored-by: Thomas <ThomasLaPiana@users.noreply.github.com>
Co-authored-by: Allison King <allison@ethyca.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Backend] Save Cookie IDs on Privacy Declarations and Surface with Embedded Notices
3 participants